Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show constraint sources in dependency solver errors #10524

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

9999years
Copy link
Collaborator

@9999years 9999years commented Nov 4, 2024

Before:

[__0] rejecting: memory-0.18.0 (constraint from user target requires ==0.17.0)

After:

[__0] rejecting: memory-0.18.0
  (constraint from cabal.project requires ==0.17.0)

First shot at #10519. Seems unavoidably large.

Builds off of #9578.

Future work: Propagate line numbers into these constraints!

Implementation strategy:

  • Propagate provenance (ProjectConfigPath/ConstraintSource) into project config types (Distribution.Client.ProjectConfig, D.C.ProjectConfig.Legacy)

  • Extract and attach that information in D.C.ProjectConfig.findProjectPackages

  • Attach that information to ProjectPackageLocation (returned from findProjectPackages). Note: Might be worth unifying that with PackageLocation, would make the new type PackageLocationProvenance.

  • Use that information to write better ConstraintSources into the solver; this is mostly implemented.

Overall it seems like all the information exists, but it takes a lot of work to propagate it into the right parts of the system.

@9999years 9999years force-pushed the dependency-provenance branch 2 times, most recently from 4811a3c to 0c8471d Compare November 4, 2024 22:29
9999years added a commit to MercuryTechnologies/cabal that referenced this pull request Nov 4, 2024
We have a lot of `showType` functions that are effectively a `Pretty`
instance but less composable. Let's make them proper `Pretty` instances.

Split off of haskell#10524
@9999years 9999years force-pushed the dependency-provenance branch 9 times, most recently from 9ebbba8 to 522bec9 Compare November 9, 2024 00:25
@ulysses4ever
Copy link
Collaborator

@jasagredo just a heads-up: on the last Cabal meeting @9999years asked to not comment on their PRs while a PR is in the draft mode. I remember it because I’m guilty in it more than many!

@9999years 9999years changed the title [WIP] Show constraint sources in dependency solver errors Show constraint sources in dependency solver errors Nov 18, 2024
@9999years 9999years marked this pull request as ready for review November 18, 2024 19:59
@9999years 9999years force-pushed the dependency-provenance branch 2 times, most recently from c10bd7d to 9854c9d Compare November 18, 2024 20:37
@9999years
Copy link
Collaborator Author

Needs a release note.

Before:

    [__0] rejecting: memory-0.18.0 (constraint from user target requires ==0.17.0)

After:

    [__0] rejecting: memory-0.18.0
      (constraint from cabal.project requires ==0.17.0)
@9999years
Copy link
Collaborator Author

Added a release note and tests, addressed comments. Should be ready to go!

Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pity 3.14.1.0 is breathing down our necks; it would be nice to see this go in, seems like it'd be a big UX improvement.

cabal-install/cabal-install.cabal Show resolved Hide resolved
import Text.PrettyPrint

-- | A package bundled with a `ConstraintSource`.
data WithConstraintSource pkg =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this type being used as WithContraintSource UserConstraint so I think pkg is not actually a pkg? i.e. constraintPackage is misnamed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, constraintPackage is often (but not always) a package. I wasn't sure of a better name for it. constraintInner, maybe? constraintConstraint is also somewhat misleadying, as it's a constraint source and not a UserConstraint itself...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed them, how do these look?

  • constraintPackageconstraintInner
  • constraintConstraintconstraintSource

cabal-install/src/Distribution/Client/CmdBuild.hs Outdated Show resolved Hide resolved
`constraintPackage` -> `constraintInner`
`constraintConstraint` -> `constraintSource`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants